Skip to content

Add localization keyword documentation - #13936

Merged
SAKavli merged 5 commits into
equinor:mainfrom
SAKavli:add-localization-keyword-documentation
Aug 20, 2026
Merged

Add localization keyword documentation#13936
SAKavli merged 5 commits into
equinor:mainfrom
SAKavli:add-localization-keyword-documentation

Conversation

@SAKavli

@SAKavli SAKavli commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #12671

Approach
Short description of the approach

(Screenshot of new behavior in GUI if applicable)

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@SAKavli SAKavli added the release-notes:documentation PR with only changes to documentation label Jul 3, 2026
@SAKavli
SAKavli force-pushed the add-localization-keyword-documentation branch from c9ebe10 to b6f4bdd Compare July 3, 2026 06:34
@codecov-commenter

codecov-commenter commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.97%. Comparing base (293883c) to head (ca82eb8).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13936      +/-   ##
==========================================
- Coverage   91.97%   91.97%   -0.01%     
==========================================
  Files         484      484              
  Lines       33644    33672      +28     
==========================================
+ Hits        30945    30969      +24     
- Misses       2699     2703       +4     
Flag Coverage Δ
cli-tests 36.49% <ø> (+0.07%) ⬆️
fuzz 44.36% <ø> (-0.03%) ⬇️
gui-tests 58.68% <ø> (-0.03%) ⬇️
performance-and-unit-tests 80.98% <ø> (+<0.01%) ⬆️
test 45.98% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 9 files with indirect coverage changes

@codspeed-hq

codspeed-hq Bot commented Jul 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing SAKavli:add-localization-keyword-documentation (ca82eb8) with main (6078659)

Open in CodSpeed

configuration. These observations are therefore already correctly configured for distance
based localization.

The radius for RFTs will be the default radius value of 3000m, but can be overwritten by

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can EAST and NORTH be overwritten too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For normal observation declarations, e.g. from_dict from obs config: yes
For observations loaded from csv: no

For normal observations, I now see that there is a "race condition" of what will be the north and east values, based on what is processed first:

        for key, value in observation_dict.items():
            match key:
                case "type" | "name":
                    pass
                case "WELL":
                    well = value
                case "PROPERTY":
                    observed_property = value
                case "VALUE":
                    observed_value = validate_float(value, key)
                case "ERROR":
                    error = validate_float(value, key)
                case "DATE":
                    date = value
                case "NORTH":
                    north = validate_float(value, key)
                case "EAST":
                    east = validate_float(value, key)
                case "TVD":
                    tvd = validate_float(value, key)
                case "CSV":
                    csv_filename = value
                case "ZONE":
                    zone = value
                case "MD":
                    md = validate_float(value, key)
                case "LOCALIZATION":
                    validate_rft_localization(value, observation_dict.context)
                    east, north, radius = extract_localization_values(value)
                    radius = (
                        radius if radius is not None else DEFAULT_LOCALIZATION_RADIUS
                    )
                case _:
                    raise _unknown_key_error(str(key), observation_dict.context)

Not great perhaps.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked this, validate_rft_localization raises an error if NORTH or EAST are defined under the LOCALIZATION block

@SAKavli
SAKavli force-pushed the add-localization-keyword-documentation branch from b6f4bdd to fa7c1d4 Compare August 19, 2026 13:11

LOCALIZATION contains two required fields: NORTH and EAST, and optionally
RADIUS. These localization attributes will be applied to all observations
sharing the same well name as the WELL configuration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe an example how to specify the well inside an observation.

@SAKavli
SAKavli force-pushed the add-localization-keyword-documentation branch 2 times, most recently from 02722fd to 3ffee1a Compare August 19, 2026 14:23

@xjules xjules left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @SAKavli! 🚀

It is unclear what this was trying to describe, but it's obvious
that it has lost its example and has therefore lost its purpose.
The LOCALIZATION keyword is already referenced in the documentation for the
bulk config without existing, so its time to add some documentation for this
keyword.
This will link to the BREAKTHROUGH observation section and the
LOCALIZATION keyword section in the places where they are
referenced from the bulk configuration documentation.
It's nice to have all the observation types gathered in one place.
Sections which are appending information to an existing section
looks nice in the table of contents on the sidebar.

They appear indented under the main section, in this case
summary observations.
@SAKavli
SAKavli force-pushed the add-localization-keyword-documentation branch from 3ffee1a to ca82eb8 Compare August 20, 2026 07:41
@SAKavli

SAKavli commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

I have force pushed to update to the commit bodies.

@SAKavli
SAKavli enabled auto-merge (rebase) August 20, 2026 07:43
@SAKavli
SAKavli merged commit e15880f into equinor:main Aug 20, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:documentation PR with only changes to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Write documentation regarding localization keywords in config

3 participants